Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide ability to exclude resource_types, instead of listing everything not excluded #9756

Merged
merged 11 commits into from
Mar 13, 2024

Conversation

gshank
Copy link
Contributor

@gshank gshank commented Mar 12, 2024

resolves #9237

Problem

People want to exclude unit_tests from running in the "build" command without specifying the complete list of all non-excluded resource types.

Solution

We've added a new cli param, --exclude-resource-types, also settable via the environment variable DBT_EXCLUDE_RESOURCE_TYPES. For consistency we've also added an environment variable for --resource-types, DBT_RESOURCE_TYPES.

Docs

We need to add some additional rows in the about-global-configs documentation page, for --resource-types/DBT_RESOURCE_TYPES and --exclude-resource-types/DBT_EXCLUDE_RESOURCE_TYPES. These params are used by the build, list, and clone commands.

Checklist

  • I have read the contributing guide and understand what's expected of me
  • I have run this code in development and it appears to resolve the stated issue
  • This PR includes tests, or tests are not required/relevant for this PR
  • This PR has no interface changes (e.g. macros, cli, logs, json artifacts, config files, adapter interface, etc) or this PR has already received feedback and approval from Product or DX
  • This PR includes type annotations for new and modified functions

@gshank gshank added the user docs [docs.getdbt.com] Needs better documentation label Mar 12, 2024
@gshank gshank requested a review from a team as a code owner March 12, 2024 18:25
@cla-bot cla-bot bot added the cla:yes label Mar 12, 2024
Copy link

codecov bot commented Mar 12, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.12%. Comparing base (8a395e9) to head (4790a75).
Report is 4 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #9756      +/-   ##
==========================================
+ Coverage   88.07%   88.12%   +0.04%     
==========================================
  Files         178      178              
  Lines       22445    22460      +15     
==========================================
+ Hits        19768    19792      +24     
+ Misses       2677     2668       -9     
Flag Coverage Δ
integration 85.50% <100.00%> (-0.11%) ⬇️
unit 62.35% <38.23%> (+0.13%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.


return list(values)
resource_types = [NodeType(rt) for rt in resource_types if rt in REFABLE_NODE_TYPES]
Copy link
Contributor

@MichelleArk MichelleArk Mar 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

perhaps we could do this prior to returning from resource_types_from_args so its not necessary here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the point of this is not so much converting to NodeType (which I did move to "resource_types_from_args) but limiting it to REFABLE_NODE_TYPES. Otherwise somebody could specify a resource_type that's not actually clonable.



def resource_types_from_args(
args, all_resource_values: Set[str], default_resource_values: Set[str]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are all_resource_values and default_resource_values actually Set[NodeType]s?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. Good catch. I've redefined them. As a sidenote, I'm always surprised by what mypy does and does not catch. I would have though it would complain about that. Maybe it automatically converts enums to strings?

Copy link
Contributor

@MichelleArk MichelleArk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great overall, the refactor is satisfying here! Just some comments on whether we can tighten up the typing on resource_types_from_args

@gshank gshank requested a review from MichelleArk March 13, 2024 15:09
@gshank gshank merged commit d65bae5 into main Mar 13, 2024
58 checks passed
@gshank gshank deleted the gs/exclude_resource_types branch March 13, 2024 20:17
@FishtownBuildBot
Copy link
Collaborator

Opened a new issue in dbt-labs/docs.getdbt.com: dbt-labs/docs.getdbt.com#5073

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla:yes user docs [docs.getdbt.com] Needs better documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CT-3467] add environment variable / flag to exclude certain resource types
3 participants